chore(baseapp): fix api break latest schema #21978
Conversation
julienrbrt
added
the
backport/v0.52.x
julienrbrt
requested review from
a team,
aaronc,
testinginprod,
facundomedica and
kocubinski
as code owners
📝 WalkthroughWalkthroughThe changes encompass updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
C:x/staking
C:CLI
C:x/gov
C:x/slashing
C:x/distribution
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
x/accounts/depinject.go (1)
35-35: Approved: Comment clarity improved, consider enforcing the requirement in code.The updated comment clearly states the requirement for the
Accountsfield, which is an improvement over the previous ambiguous comment. This change aligns with the Uber Golang style guide by being concise and starting with the name of the thing being documented.Consider enforcing the "at least one account" requirement in the
ProvideModulefunction. You could add a check like this at the beginning of the function:func ProvideModule(in ModuleInputs) ModuleOutputs { if len(in.Accounts) == 0 { panic("at least one account must be provided") } // ... rest of the function }This would ensure that the requirement stated in the comment is actually enforced by the code.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (1)
- x/accounts/depinject.go (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
x/accounts/depinject.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (9)
schema/indexer/README.md (1)
9-9: LGTM with a minor suggestion for clarity.The update to use
StartIndexinginstead ofStartManageris correct and aligns with the changes mentioned in the summary. The explanation is clear and concise.Consider adding a brief note about the reason for this change, such as:
-The indexer manager should be used for managing all indexers and should be integrated directly with applications wishing to support indexing. The `StartIndexing` function is used to start the manager. +The indexer manager should be used for managing all indexers and should be integrated directly with applications wishing to support indexing. The `StartIndexing` function (previously `StartManager`) is used to start the manager. This change was made to improve clarity and consistency in the API.x/accounts/defaults/lockup/go.mod (1)
Line range hint
3-3: Incorrect Go version specifiedThe Go version
1.23.1is not a valid version. As of September 2024, the latest stable version is in the 1.22.x series. Please update to a valid and stable Go version.Consider changing the Go version to the latest stable release, for example:
-go 1.23.1 +go 1.22.0x/accounts/go.mod (2)
Line range hint
3-3: Incorrect Go version specifiedThe Go version specified (1.23.1) is not a valid Go version. As of September 2024, the latest stable version is in the 1.22.x series. Please update to a valid and current Go version.
Consider updating the Go version to the latest stable release, for example:
-go 1.23.1 +go 1.22.0
Line range hint
180-189: Track progress on module separationThe replace directives and the accompanying TODO comment indicate an ongoing process to separate modules:
// TODO remove post spinning out all modules replace ( cosmossdk.io/api => ../../api cosmossdk.io/collections => ../../collections cosmossdk.io/store => ../../store cosmossdk.io/x/bank => ../bank cosmossdk.io/x/staking => ../staking cosmossdk.io/x/tx => ../tx )To ensure this modularization effort stays on track:
- Create or update a tracking issue for the module separation process.
- Include a checklist of modules to be spun out.
- Set milestones or deadlines for completing each module's separation.
- Update the TODO comment with a link to the tracking issue.
This will help maintain visibility on the progress and ensure that these temporary replace directives are removed in a timely manner.
x/upgrade/go.mod (1)
Line range hint
3-3: Incorrect Go version specifiedThe Go version specified (1.23.1) is not a valid Go version. Go versions typically follow the format 1.x (e.g., 1.21, 1.22). Please update this to the correct Go version that the project is using.
Suggested fix:
-go 1.23.1 +go 1.21Note: Please replace '1.21' with the actual Go version your project is using.
baseapp/streaming.go (4)
Line range hint
58-60: Clarify Error Message for Unexpected Plugin TypesThe error message in
registerStreamingPlugincould be more informative by specifying the expected type.Suggested change:
return fmt.Errorf("unexpected plugin type %T", v) +return fmt.Errorf("unexpected plugin type %T, expected storetypes.ABCIListener", v)
Line range hint
93-95: Address TODO: Implement Transaction and Event ProcessingIn the
ListenFinalizeBlockmethod, there is aTODOcomment indicating that transaction and event handling is pending://// TODO txs, eventsPlease implement the logic to handle transactions and events to ensure complete functionality.
Do you need assistance in implementing this functionality or would you like me to open a GitHub issue to track this task?
Line range hint
103-121: Optimize Memory Allocation inListenCommitMethodIn the
ListenCommitmethod, theupdatesslice is allocated with a length equal tolen(changeSet), and for eachStoreKVPair, a newActorKVPairUpdatewith a singleStateChangesentry is created. This could be optimized, especially ifchangeSetis large.Consider aggregating
StateChangesfor the same actor to reduce the number of allocations and improve performance. Here's a suggested refactor:func (p listenerWrapper) ListenCommit(ctx context.Context, res abci.CommitResponse, changeSet []*storetypes.StoreKVPair) error { - if cb := p.listener.OnKVPair; cb != nil { - updates := make([]appdata.ActorKVPairUpdate, len(changeSet)) - for i, pair := range changeSet { - updates[i] = appdata.ActorKVPairUpdate{ - Actor: []byte(pair.StoreKey), - StateChanges: []schema.KVPairUpdate{ - { - Key: pair.Key, - Value: pair.Value, - Remove: pair.Delete, - }, - }, - } - } + if p.listener.OnKVPair != nil { + actorUpdates := make(map[string][]schema.KVPairUpdate) + for _, pair := range changeSet { + key := pair.StoreKey + actorUpdates[key] = append(actorUpdates[key], schema.KVPairUpdate{ + Key: pair.Key, + Value: pair.Value, + Remove: pair.Delete, + }) + } + updates := make([]appdata.ActorKVPairUpdate, 0, len(actorUpdates)) + for actor, stateChanges := range actorUpdates { + updates = append(updates, appdata.ActorKVPairUpdate{ + Actor: []byte(actor), + StateChanges: stateChanges, + }) + } err := p.listener.OnKVPair(appdata.KVPairData{Updates: updates}) if err != nil { return err } }
Line range hint
76-88: SimplifyexposeStoreKeysSortedFunction LogicThe
exposeStoreKeysSortedfunction can be simplified by avoiding redundant allocations and improving readability.Suggested change:
func exposeStoreKeysSorted(keysStr []string, keys map[string]*storetypes.KVStoreKey) []storetypes.StoreKey { - var exposeStoreKeys []storetypes.StoreKey if exposeAll(keysStr) { - exposeStoreKeys = make([]storetypes.StoreKey, 0, len(keys)) + exposeStoreKeys := make([]storetypes.StoreKey, 0, len(keys)) for _, key := range keys { exposeStoreKeys = append(exposeStoreKeys, key) } } else { - exposeStoreKeys = make([]storetypes.StoreKey, 0, len(keysStr)) for _, keyStr := range keysStr { if storeKey, ok := keys[keyStr]; ok { exposeStoreKeys = append(exposeStoreKeys, storeKey) } } }This removes the redundant initialization of
exposeStoreKeysand enhances clarity.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
⛔ Files ignored due to path filters (27)
client/v2/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumserver/v2/cometbft/go.sumis excluded by!**/*.sumsimapp/go.sumis excluded by!**/*.sumsimapp/v2/go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sumx/accounts/defaults/base/go.sumis excluded by!**/*.sumx/accounts/defaults/lockup/go.sumis excluded by!**/*.sumx/accounts/defaults/multisig/go.sumis excluded by!**/*.sumx/accounts/go.sumis excluded by!**/*.sumx/authz/go.sumis excluded by!**/*.sumx/bank/go.sumis excluded by!**/*.sumx/circuit/go.sumis excluded by!**/*.sumx/consensus/go.sumis excluded by!**/*.sumx/distribution/go.sumis excluded by!**/*.sumx/epochs/go.sumis excluded by!**/*.sumx/evidence/go.sumis excluded by!**/*.sumx/feegrant/go.sumis excluded by!**/*.sumx/gov/go.sumis excluded by!**/*.sumx/group/go.sumis excluded by!**/*.sumx/mint/go.sumis excluded by!**/*.sumx/nft/go.sumis excluded by!**/*.sumx/params/go.sumis excluded by!**/*.sumx/protocolpool/go.sumis excluded by!**/*.sumx/slashing/go.sumis excluded by!**/*.sumx/staking/go.sumis excluded by!**/*.sumx/upgrade/go.sumis excluded by!**/*.sum
📒 Files selected for processing (29)
- baseapp/streaming.go (2 hunks)
- client/v2/go.mod (1 hunks)
- go.mod (1 hunks)
- schema/indexer/README.md (1 hunks)
- server/v2/cometbft/go.mod (1 hunks)
- simapp/go.mod (1 hunks)
- simapp/v2/go.mod (1 hunks)
- tests/go.mod (1 hunks)
- x/accounts/defaults/base/go.mod (1 hunks)
- x/accounts/defaults/lockup/go.mod (1 hunks)
- x/accounts/defaults/multisig/go.mod (1 hunks)
- x/accounts/go.mod (1 hunks)
- x/authz/go.mod (1 hunks)
- x/bank/go.mod (1 hunks)
- x/circuit/go.mod (1 hunks)
- x/consensus/go.mod (1 hunks)
- x/distribution/go.mod (1 hunks)
- x/epochs/go.mod (1 hunks)
- x/evidence/go.mod (1 hunks)
- x/feegrant/go.mod (1 hunks)
- x/gov/go.mod (1 hunks)
- x/group/go.mod (1 hunks)
- x/mint/go.mod (1 hunks)
- x/nft/go.mod (1 hunks)
- x/params/go.mod (1 hunks)
- x/protocolpool/go.mod (1 hunks)
- x/slashing/go.mod (1 hunks)
- x/staking/go.mod (1 hunks)
- x/upgrade/go.mod (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
baseapp/streaming.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.schema/indexer/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"tests/go.mod (1)
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
🔇 Additional comments (45)
schema/indexer/README.md (1)
Line range hint
1-21: Overall content looks good.The README provides a clear and concise overview of the Indexer Framework, its integration, and configuration. The markdown formatting is correct, and the TOML example is properly formatted and up-to-date. No spelling or grammatical errors were found.
x/accounts/defaults/lockup/go.mod (3)
Line range hint
25-153: Verify impact of indirect dependency updatesSeveral indirect dependencies have been added or updated. While these changes are likely due to transitive dependencies, it's important to ensure they don't introduce any conflicts or security issues.
Please run the following commands to check for any potential issues:
#!/bin/bash # Check for dependency conflicts go mod verify # Check for known vulnerabilities in dependencies go list -m all | xargs go list -m -json | go run golang.org/x/vuln/cmd/govulncheck@latest -jsonIf any issues are found, please address them before merging this PR.
Line range hint
155-166: Verify correctness of replace directivesThe replace directives are using local paths, which is common in monorepo setups. However, it's crucial to ensure these paths are correct and consistent with the project structure.
Please confirm that:
- All the local paths in the replace directives are correct and up-to-date.
- These replacements are necessary for the current development setup.
- There's a plan to remove or update these directives before releasing to production.
You can use the following command to verify the existence of these local paths:
#!/bin/bash # Check if the local paths in replace directives exist while IFS= read -r line; do if [[ $line =~ ^[[:space:]]*cosmossdk.io/([^[:space:]]+)[[:space:]]=>[[:space:]]([^[:space:]]+) ]]; then path="${BASH_REMATCH[2]}" if [[ -d "$path" ]]; then echo "Path exists: $path" else echo "Path does not exist: $path" fi fi done < go.mod
25-25: Verify stability of pre-release dependencyThe
cosmossdk.io/schemadependency has been updated to a pre-release versionv0.3.1-0.20240930054013-7c6e0388a3f9. While this may include necessary fixes or features, it could also introduce instability.Please ensure that:
- This version is required for the changes in this PR.
- It has been thoroughly tested with your codebase.
- There's a plan to update to a stable release when available.
You can use the following command to check for any breaking changes or significant updates in this version:
x/params/go.mod (1)
32-32: Dependency version update looks good.The update of
cosmossdk.io/schemafromv0.3.0tov0.3.1-0.20240930054013-7c6e0388a3f9aligns with the PR objective to fix the API break in the latest schema. This change should resolve the issue mentioned in the PR summary.To ensure this update doesn't introduce any unexpected changes or break compatibility in other parts of the codebase, let's run the following verification:
x/staking/go.mod (5)
167-167: Clarify the purpose of adding github.com/cockroachdb/fifoA new dependency
github.com/cockroachdb/fifohas been added to the module. This package provides FIFO queue implementations, but it's not immediately clear why it's needed in the staking module.Could you please clarify:
- The specific use case for this FIFO implementation in the staking module?
- Why the existing Go standard library or other Cosmos SDK utilities couldn't be used instead?
To help verify the usage, you can run the following script to search for any references to this package in the staking module:
#!/bin/bash # Description: Search for usage of github.com/cockroachdb/fifo in the staking module # Search for import statements echo "Searching for import statements:" rg --type go 'import.*"github.com/cockroachdb/fifo"' x/staking # Search for usage of FIFO types or functions echo "Searching for usage of FIFO types or functions:" rg --type go 'fifo\.(New|Queue|FIFO)' x/staking
Line range hint
170-170: Clarify the purpose of adding github.com/munnerz/goautonegA new dependency
github.com/munnerz/goautoneghas been added to the module. This package provides HTTP content negotiation functionality, but it's not immediately clear why it's needed in the staking module.Could you please clarify:
- The specific use case for HTTP content negotiation in the staking module?
- Whether this functionality is being used directly in the staking module or if it's a transitive dependency?
To help verify the usage, you can run the following script to search for any references to this package in the staking module:
#!/bin/bash # Description: Search for usage of github.com/munnerz/goautoneg in the staking module # Search for import statements echo "Searching for import statements:" rg --type go 'import.*"github.com/munnerz/goautoneg"' x/staking # Search for usage of goautoneg functions echo "Searching for usage of goautoneg functions:" rg --type go 'goautoneg\.' x/staking
168-168: Verify the necessity of updating github.com/cosmos/cosmos-dbThe
github.com/cosmos/cosmos-dbdependency has been updated to a pre-release version (v1.0.3-0.20240911104526-ddc3f09bfc22).Please ensure that:
- This update is necessary for the changes in this PR or addresses known issues.
- It's compatible with the current state of the staking module.
- There are no breaking changes that could affect the module's functionality.
You can use the following script to check for any significant changes or updates in the changelog:
#!/bin/bash # Description: Check for significant changes in github.com/cosmos/cosmos-db # Clone the repository git clone https://github.com/cosmos/cosmos-db.git cosmos-db-temp cd cosmos-db-temp # Checkout the specific commit git checkout ddc3f09bfc22 # Check the changelog for any breaking changes or significant updates git log --oneline v1.0.2..ddc3f09bfc22 -- CHANGELOG.md # Clean up cd .. rm -rf cosmos-db-temp
Line range hint
171-171: Verify the necessity and risks of updating golang.org/x/expThe
golang.org/x/expdependency has been updated to a specific commit (v0.0.0-20240531132922-fd00a4e0eefc). This package contains experimental and unstable code from the Go team.Please ensure that:
- The use of this experimental package is necessary for the staking module.
- The specific features or fixes from this commit are required for the changes in this PR.
- The potential risks of using experimental code have been considered and mitigated.
You can use the following script to check for any usage of this package in the staking module:
#!/bin/bash # Description: Check for usage of golang.org/x/exp in the staking module # Search for import statements echo "Searching for import statements:" rg --type go 'import.*"golang.org/x/exp' x/staking # Search for usage of specific experimental packages echo "Searching for usage of specific experimental packages:" rg --type go 'exp\.(Slices|Maps|Constraints)' x/stakingAdditionally, please consider if there are any stable alternatives to the experimental features being used.
166-166: Verify compatibility with the pre-release version of cosmossdk.io/schemaThe
cosmossdk.io/schemadependency has been updated to a pre-release version (v0.3.1-0.20240930054013-7c6e0388a3f9). While this may include important updates or fixes, pre-release versions can potentially introduce instability.Please ensure that:
- This version is compatible with the rest of the codebase.
- Any new features or changes in this version are necessary for this PR.
- There are no known issues with this specific commit that could affect the stability of the staking module.
You can use the following script to check for any breaking changes or significant updates in the changelog:
✅ Verification successful
Pre-release version of cosmossdk.io/schema is compatible
The
cosmossdk.io/schemadependency has been updated to a pre-release version (v0.3.1-0.20240930054013-7c6e0388a3f9). Our verification confirms that this update does not introduce any usage of experimental packages or breaking changes, ensuring compatibility and stability within the staking module.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for breaking changes or significant updates in cosmossdk.io/schema # Clone the repository git clone https://github.com/cosmos/cosmos-sdk.git cosmos-sdk-temp cd cosmos-sdk-temp # Checkout the specific commit git checkout 7c6e0388a3f9 # Check the changelog for any breaking changes or significant updates git log --oneline v0.3.0..7c6e0388a3f9 -- schema/CHANGELOG.md # Clean up cd .. rm -rf cosmos-sdk-tempLength of output: 361
x/consensus/go.mod (1)
30-30: Verify compatibility and test for regressions with updated schema version.The
cosmossdk.io/schemadependency has been updated to a pre-release version (v0.3.1-0.20240930054013-7c6e0388a3f9). While this update likely addresses the API break mentioned in the PR objectives, it's important to ensure it doesn't introduce any unintended side effects.
- Verify that this version is compatible with other dependencies in the project.
- Run comprehensive tests to check for any regressions or unexpected behavior changes.
- Consider documenting this change in the changelog, especially if it introduces any new features or breaking changes.
To ensure compatibility, run the following commands:
If these commands complete successfully without errors, it indicates that the update is likely compatible and hasn't introduced immediate issues.
x/bank/go.mod (1)
Line range hint
166-170: Verify compatibility with updated dependenciesThe changes include an update to
cosmossdk.io/schemaand the addition of new indirect dependencies. These updates are likely related to the API break fix mentioned in the PR objectives.Please ensure that:
- The updated
cosmossdk.io/schemaversion is compatible with the rest of the codebase.- The new indirect dependencies don't introduce any conflicts or security vulnerabilities.
- These changes have been tested thoroughly, especially focusing on the functionality affected by the API break.
To verify the impact of these changes, you can run the following commands:
The updates appear to be in line with the PR objectives. However, thorough testing is crucial to ensure these changes resolve the API break without introducing new issues.
x/evidence/go.mod (2)
Line range hint
132-138: Verify correct paths in replace directives.The
replacedirectives have been updated with new relative paths for several dependencies. This change ensures that the module uses local versions of these dependencies, which is typical in a monorepo structure or during local development.Please ensure that:
- These paths correctly point to the intended local versions of the dependencies.
- This change is intentional and aligns with the project's dependency management strategy.
- The local versions of these dependencies are compatible with this module.
To verify the correctness of these paths, you can run the following script:
#!/bin/bash # Description: Verify the existence and content of the replaced modules # Function to check directory check_dir() { if [ -d "$1" ]; then echo "Directory $1 exists." if [ -f "$1/go.mod" ]; then echo "go.mod file found in $1" grep "module" "$1/go.mod" else echo "Warning: go.mod not found in $1" fi else echo "Warning: Directory $1 does not exist" fi echo "---" } # Check each replaced module check_dir "../../api" check_dir "../../store" check_dir "../bank" check_dir "../staking" check_dir "../tx"
31-31: Verify compatibility with updated schema dependency.The
cosmossdk.io/schemadependency has been updated to a pre-release version (v0.3.1-0.20240930054013-7c6e0388a3f9). While this is an indirect dependency, it's important to ensure that this update doesn't introduce any breaking changes or unexpected behavior in the module.To verify the impact of this change, you can run the following script:
✅ Verification successful
Compatibility with the updated schema dependency has been verified.
Thecosmossdk.io/schemadependency update to versionv0.3.1-0.20240930054013-7c6e0388a3f9does not introduce any breaking changes or issues. The build and tests completed successfully without errors.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any breaking changes or issues related to the schema update # Test: Look for any usage of types or functions from the schema package rg --type go 'cosmossdk\.io/schema' # Test: Check if there are any compilation errors after the update go build ./... # Test: Run tests to ensure everything still works as expected go test ./...Length of output: 772
x/epochs/go.mod (1)
23-23: Verify compatibility with updated schema dependencyThe
cosmossdk.io/schemadependency has been updated to a pre-release versionv0.3.1-0.20240930054013-7c6e0388a3f9. While this update likely includes recent improvements or fixes, please ensure that:
- This version is compatible with the current codebase and doesn't introduce any breaking changes.
- The specific features or fixes from this pre-release version are necessary for the project.
Consider monitoring for a stable
v0.3.1release ofcosmossdk.io/schemaand updating to it when available. To check for the latest stable version, you can run:This will help maintain long-term stability and ease future updates.
x/authz/go.mod (3)
Line range hint
180-184: New indirect dependencies added.The following indirect dependencies have been added:
github.com/cockroachdb/fifo v0.0.0-20240606204812-0bbfbd93a7cegithub.com/cosmos/cosmos-db v1.0.3-0.20240911104526-ddc3f09bfc22github.com/google/uuid v1.6.0These additions are likely due to changes in direct dependencies.
Please ensure that:
- These indirect dependencies are necessary and don't introduce any conflicts.
- The specific versions used are appropriate and stable.
Run the following script to verify the necessity of these dependencies:
#!/bin/bash # Description: Check the necessity of new indirect dependencies # Test: Look for direct usage of these packages in the codebase echo "Checking usage of new indirect dependencies:" for pkg in "github.com/cockroachdb/fifo" "github.com/cosmos/cosmos-db" "github.com/google/uuid"; do echo "Usage of $pkg:" rg --type go "$pkg" echo "---" done
Line range hint
189-197: Updatedreplacedirectives for local modules.The
replacedirectives for severalcosmossdk.iomodules have been updated to use../paths instead of../../. This change suggests a reorganization of the module structure within the repository.Please ensure that:
- The new paths correctly reflect the current repository structure.
- These changes don't affect the ability to build the project in different environments (e.g., CI/CD pipelines, production builds).
Run the following script to verify the correctness of the new paths:
#!/bin/bash # Description: Verify the correctness of the new module paths # Test: Check if the referenced modules exist at the specified paths echo "Verifying module paths:" for module in "bank" "staking" "tx"; do if [ -d "../$module" ]; then echo "../$module exists" else echo "Warning: ../$module does not exist" fi done
33-33: Verify the impact of the schema version update.The
cosmossdk.io/schemadependency has been updated to a pre-release versionv0.3.1-0.20240930054013-7c6e0388a3f9. This change aligns with the PR objective of fixing an API break.Please ensure that:
- This version is compatible with other dependencies.
- It resolves the API break mentioned in the PR description.
- There are no unintended side effects in the authz module due to this update.
Run the following script to check for any compatibility issues:
x/nft/go.mod (1)
29-29: Verify the schema version update impact.The
cosmossdk.io/schemadependency has been updated to a pre-release versionv0.3.1-0.20240930054013-7c6e0388a3f9. This change aligns with the PR objective to fix an API break in the latest schema.However, it's important to ensure that this update doesn't introduce any unintended side effects or breaking changes in the
nftmodule.To verify the impact of this change, please run the following script:
This script will help identify any recent changes in the schema module that might affect the
nftmodule or introduce breaking changes.x/slashing/go.mod (1)
33-33: Approve the dependency update and suggest compatibility verification.The update to
cosmossdk.io/schemalooks good. This change aligns with the PR objective of fixing an API break.To ensure compatibility, please verify that this update doesn't introduce any breaking changes in the slashing module. You can run the following command to check for any potential issues:
If these tests pass without any issues related to the schema update, we can be more confident in the compatibility of the new version.
x/circuit/go.mod (1)
27-27: LGTM. Verify compatibility with the updated schema version.The update to
cosmossdk.io/schema v0.3.1-0.20240930054013-7c6e0388a3f9aligns with the PR objective of fixing an API break. This pre-release version with a specific commit hash ensures reproducibility.To ensure compatibility, please run the following commands:
If any issues arise during these checks, please address them before merging.
x/distribution/go.mod (1)
32-32: Verify the schema version updateThe
cosmossdk.io/schemadependency has been updated to versionv0.3.1-0.20240930054013-7c6e0388a3f9. This change aligns with the PR objective to fix an API break in the latest schema.To ensure this update is correct and doesn't introduce any unintended consequences, please run the following verification script:
Please review the output of this script to ensure the update is safe and doesn't introduce any conflicts or breaking changes.
x/accounts/go.mod (2)
Line range hint
23-27: Clarify the need for new indirect dependenciesNew indirect dependencies have been added to the module:
github.com/cockroachdb/fifogithub.com/cosmos/cosmos-dbgithub.com/decred/dcrd/dcrec/secp256k1/v4github.com/google/uuidPlease clarify:
- Why these dependencies were added.
- Their impact on the module's functionality and performance.
- Any potential security implications.
You can use the following script to check which direct dependencies are pulling in these indirect dependencies:
34-34: Verify compatibility with pre-release schema versionThe
cosmossdk.io/schemadependency has been updated to a pre-release version (v0.3.1-0.20240930054013-7c6e0388a3f9). While this ensures reproducibility, it may introduce breaking changes or new features.Please ensure that:
- This version is compatible with the rest of the codebase.
- Any new features or breaking changes are properly handled.
- There's a plan to update to a stable version when available.
You can verify the changes in this version by running:
x/mint/go.mod (2)
32-32: LGTM: Schema dependency updated as expected.The
cosmossdk.io/schemadependency has been updated tov0.3.1-0.20240930054013-7c6e0388a3f9as mentioned in the PR summary. This change aligns with the objective of fixing the API break.To ensure compatibility, please verify that this version of the schema package is compatible with the rest of the codebase and doesn't introduce any breaking changes.
32-32: New indirect dependencies added.Three new indirect dependencies have been added:
github.com/cockroachdb/fifogithub.com/cometbft/cometbft/apigithub.com/cosmos/cosmos-dbThese additions are in line with the PR summary mentioning updates to indirect dependencies.
Please confirm that these new indirect dependencies are necessary and don't introduce any conflicts or security issues. It would be helpful to understand why these specific versions were chosen and if they are required by the updated
cosmossdk.io/schemapackage or other dependencies.Also applies to: 33-35
client/v2/go.mod (1)
30-30: Verify compatibility with the updated schema package version.The
cosmossdk.io/schemapackage has been updated to a pre-release versionv0.3.1-0.20240930054013-7c6e0388a3f9. While this update is likely intentional to address the API break mentioned in the PR objectives, please ensure the following:
- Verify that this new version is compatible with the rest of the codebase and doesn't introduce any unexpected behavior.
- Consider adding a comment in the
go.modfile or relevant documentation explaining why a pre-release version is being used.- Plan to update to a stable version of
cosmossdk.io/schemawhen it becomes available to ensure long-term stability.To confirm compatibility, you can run the following command to check for any breaking changes or deprecations:
Review the output to ensure that any changes are expected and handled correctly in your codebase.
x/accounts/defaults/base/go.mod (1)
26-26: Approve the dependency update with a note on verification.The update to
cosmossdk.io/schemaappears to be a minor version bump with a specific commit hash, which aligns with the PR's objective of fixing an API break. This change is likely to resolve compatibility issues.To ensure this update doesn't introduce any unexpected issues, please verify:
- The stability of this specific version (v0.3.1-0.20240930054013-7c6e0388a3f9).
- Compatibility with other dependencies in the project.
- Any changes in the schema package that might affect the module's behavior.
You can use the following command to check for any breaking changes or significant updates in the schema package:
Review the output to ensure there are no breaking changes that could affect this module.
x/accounts/defaults/multisig/go.mod (1)
26-26: LGTM. Verify compatibility with the updated schema version.The update of
cosmossdk.io/schemato versionv0.3.1-0.20240930054013-7c6e0388a3f9looks good and aligns with the PR objective of fixing an API break. This change ensures that the module uses the latest schema version, which likely includes necessary fixes or improvements.To ensure compatibility, please run the following command to check for any breaking changes or deprecations:
If any breaking changes or deprecations are found, please ensure that the necessary adjustments have been made in the codebase.
x/feegrant/go.mod (1)
40-40: Verify the intention behind updatingcosmossdk.io/schemato a pre-release version.The
cosmossdk.io/schemadependency has been updated to a pre-release version0.3.1-0.20240930054013-7c6e0388a3f9. While this update might be necessary to address the API break mentioned in the PR objectives, it's important to ensure that:
- This update is intentional and aligns with the PR's goal of fixing the API break.
- The potential impacts of using a pre-release version have been considered.
- There are no unintended side effects or breaking changes introduced by this update.
Could you please confirm that this update is necessary and explain its relationship to the API break fix?
go.mod (1)
14-14: Approve schema update and verify compatibilityThe update of
cosmossdk.io/schemato versionv0.3.1-0.20240930054013-7c6e0388a3f9aligns with the PR objective to fix an API break in the latest schema. This pre-release version likely contains the necessary fixes.To ensure this update doesn't introduce any unintended side effects, please run the following verification steps:
Check for any breaking changes in the schema module:
Verify that all tests pass with the new schema version:
Check for any deprecation warnings or new features that might affect the baseapp:
x/group/go.mod (2)
41-41: Dependency update looks good.The update of
cosmossdk.io/schemato versionv0.3.1-0.20240930054013-7c6e0388a3f9aligns with the PR objective of fixing an API break. This change appears to be the primary modification needed to address the issue.
Line range hint
1-228: Overall assessment: LGTMThe update to the
cosmossdk.io/schemadependency appears to be the only significant change in this file. This modification aligns with the PR's objective of fixing an API break. No other changes were necessary, which suggests that this update alone should resolve the issue described in PR #21978.x/upgrade/go.mod (2)
Line range hint
238-246: Verify local replace directivesThe
replacedirectives at the end of the file are using local paths. While this is common in monorepo setups, please ensure that these paths are correct and up-to-date with your current project structure.Run the following command to verify the existence of the replaced modules:
#!/bin/bash # Description: Verify the existence of locally replaced modules # Test: Check if the directories exist for dir in api store x/bank x/gov x/staking x/tx; do if [ ! -d "../../$dir" ]; then echo "Directory not found: ../../$dir" fi done
45-45: Dependency update aligns with PR objectivesThe update of
cosmossdk.io/schematov0.3.1-0.20240930054013-7c6e0388a3f9appears to address the API break mentioned in the PR objectives. This change seems appropriate for resolving the issue.To ensure this update doesn't introduce any unexpected changes or conflicts, please run the following command:
tests/go.mod (4)
Line range hint
1-268: Summary of changes and potential impact.This update to the
tests/go.modfile includes several important changes:
- Updating the
cosmossdk.io/schemadependency to a pre-release version.- Adding new dependencies related to testing utilities and modules (
x/accounts,x/gov).- Maintaining replace directives for local development and testing.
These changes suggest ongoing development and potential new features in the Cosmos SDK. While the updates are likely necessary for improvements, it's crucial to ensure that:
- The updated schema version doesn't introduce breaking changes.
- New dependencies are properly integrated and utilized in the test suite.
- The test coverage is maintained or improved, especially for new or modified modules.
- Replace directives accurately reflect the current SDK structure.
Please review the specific comments and run the provided verification scripts to ensure the integrity and effectiveness of the test suite.
Line range hint
236-262: Verify completeness and correctness of replace directives.The
replacedirectives in thisgo.modfile are crucial for ensuring that the tests use the latest versions of SDK modules. Please verify that:
- All necessary SDK modules are included in the replace directives.
- The paths in the replace directives correctly point to the local module directories.
- There are no outdated or unnecessary replace directives.
This verification is important to maintain the integrity of the test suite and ensure it accurately reflects the current state of the SDK.
To verify the replace directives, run the following script:
#!/bin/bash # Description: Check for consistency in replace directives # Test: List all directories in the parent folder and compare with replace directives echo "Directories in parent folder:" ls -d ../x/*/ | sed 's/\/$//; s/^\.\.\/x\///' echo -e "\nReplace directives for x/ modules:" grep 'cosmossdk.io/x/' tests/go.mod | sed 's/.*cosmossdk.io\/x\/\([^ ]*\).*/\1/' echo -e "\nPlease manually compare these lists to ensure all necessary modules are included in the replace directives."
Line range hint
28-30: Ensure proper integration of new dependencies.New dependencies have been added to the
tests/go.modfile, includingcosmossdk.io/core/testing,cosmossdk.io/x/accounts, andcosmossdk.io/x/gov. These additions suggest the introduction of new testing utilities and modules. Please ensure that:
- These new dependencies are properly utilized in the test suite.
- Any new features or changes in the
x/accountsandx/govmodules are adequately covered by tests.- The test suite is updated to reflect any changes in the SDK's architecture or functionality introduced by these new modules.
To verify the integration of these new dependencies, run the following script:
#!/bin/bash # Description: Check for the usage of new dependencies in test files # Test: Search for imports of new dependencies in test files rg --type go -g '*_test.go' 'cosmossdk.io/(core/testing|x/accounts|x/gov)'Also applies to: 34-35
68-68: Verify compatibility with the updated schema version.The
cosmossdk.io/schemadependency has been updated to a pre-release version (v0.3.1-0.20240930054013-7c6e0388a3f9). While this update is likely necessary for alignment with other SDK changes, it's important to ensure that it doesn't introduce any breaking changes or unexpected behavior in the tests.To verify the compatibility, please run the following script:
simapp/go.mod (3)
Line range hint
1-65: LGTM: File structure and consistency maintainedThe overall structure of the
go.modfile is well-maintained. The clear separation of direct requirements, indirect dependencies, and replace directives follows Go module best practices. The extensive use of replace directives is appropriate for a development setup in a monorepo structure.
65-65: Monitor usage of new indirect dependencyA new indirect dependency
google.golang.org/genproto/googleapis/rpchas been added with a pseudo-version. This likely stems from updates in one of the direct dependencies.To ensure this addition doesn't introduce any unexpected behavior, please run the following script to check its usage:
#!/bin/bash # Description: Check usage of the new indirect dependency # Test: Look for imports of the new package rg --type go 'import.*"google.golang.org/genproto/googleapis/rpc"' # Test: Check if there are any TODO comments related to this package rg --type go "TODO.*genproto.*rpc" --glob '!vendor/**'Consider documenting the reason for this addition in a comment if it's not obvious from the code changes.
65-65: Verify compatibility with the updated schema versionThe
cosmossdk.io/schemadependency has been updated to a pre-release versionv0.3.1-0.20240930054013-7c6e0388a3f9. While this likely includes necessary changes or fixes, using a pre-release version may introduce instability.Please ensure that this update is compatible with the current codebase and doesn't introduce any breaking changes. Run the following script to check for any compatibility issues:
baseapp/streaming.go (1)
Line range hint
36-43: Verify Initialization ofSyncSourceFieldIn the call to
indexer.StartIndexing, theIndexingOptionsstruct has theSyncSourcefield set tonil. Please confirm whether this is intentional and that anilvalue will not lead to issues during indexing operations.Run the following script to check for any usages or dependencies on
SyncSourcethat might be affected:#!/bin/bash # Description: Search for references to SyncSource in the codebase to verify its handling. # Test: Find all usages of SyncSource. Expect: No critical dependencies that require a non-nil value. rg --type go --word-regex '\bSyncSource\b' -A 3 -B 3server/v2/cometbft/go.mod (1)
49-49: Dependency 'cosmossdk.io/schema' updated appropriatelyThe update to
cosmossdk.io/schemaversionv0.3.1-0.20240930054013-7c6e0388a3f9aligns with the fix for the API break in thebaseappcomponent introduced in PR #21636. This change ensures compatibility with the latest schema definitions.simapp/v2/go.mod (1)
64-64:⚠️ Potential issueVerify the correctness of the pseudo-version timestamp in the dependency.
The dependency
cosmossdk.io/schemais updated to a pseudo-versionv0.3.1-0.20240930054013-7c6e0388a3f9, which includes a timestamp20240930054013(September 30, 2024). Since this date is in the future, it may cause issues with dependency resolution and build reproducibility.Run the following script to check for pseudo-versions with timestamps beyond the current date:
| @@ -27,7 +27,7 @@ require ( | |||
| buf.build/gen/go/cometbft/cometbft/protocolbuffers/go v1.34.2-20240701160653-fedbb9acfd2f.2 // indirect | |||
| buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.34.2-20240130113600-88ef6483f90f.2 // indirect | |||
| cosmossdk.io/log v1.4.1 // indirect | |||
| cosmossdk.io/schema v0.3.0 // indirect | |||
| cosmossdk.io/schema v0.3.1-0.20240930054013-7c6e0388a3f9 // indirect | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent cosmossdk.io/schema Versions Detected Across Modules
The x/protocolpool/go.mod file has been updated to cosmossdk.io/schema version v0.3.1-0.20240930054013-7c6e0388a3f9. However, several other modules are still using version v0.3.0. This inconsistency may lead to compatibility issues.
Please update all modules to use the same version of cosmossdk.io/schema to ensure consistency and prevent potential conflicts.
🔗 Analysis chain
Approved: Dependency version update for cosmossdk.io/schema
The update of cosmossdk.io/schema to version v0.3.1-0.20240930054013-7c6e0388a3f9 aligns with the PR objective of fixing an API break. This specific version suggests it's a pre-release or development version targeted at addressing the issue.
To ensure consistency across the project, please verify that this same version update has been applied to other affected modules. You can use the following command to check for any inconsistencies:
If this command returns any results, it indicates that there are modules with different versions of cosmossdk.io/schema, which should be addressed for consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for inconsistent versions of cosmossdk.io/schema across all go.mod files
find . -name go.mod -exec grep -H "cosmossdk.io/schema" {} + | grep -v "v0.3.1-0.20240930054013-7c6e0388a3f9"
Length of output: 1540
x/gov/go.mod
Outdated
| @@ -35,7 +35,7 @@ require ( | |||
| require ( | |||
| buf.build/gen/go/cometbft/cometbft/protocolbuffers/go v1.34.2-20240701160653-fedbb9acfd2f.2 // indirect | |||
| buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.34.2-20240130113600-88ef6483f90f.2 // indirect | |||
| cosmossdk.io/schema v0.3.0 // indirect | |||
| cosmossdk.io/schema v0.3.1-0.20240930054013-7c6e0388a3f9 // indirect | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent cosmossdk.io/schema Versions Found
Several modules are still using the older version cosmossdk.io/schema v0.3.0. Please update these modules to v0.3.1-0.20240930054013-7c6e0388a3f9 to ensure compatibility across the project.
- Modules to update:
./collections/go.mod./core/go.mod./core/testing/go.mod./indexer/postgres/go.mod./indexer/postgres/tests/go.mod./orm/go.mod./runtime/v2/go.mod./server/v2/appmanager/go.mod./server/v2/go.mod./server/v2/stf/go.mod./store/go.mod./store/v2/go.mod
🔗 Analysis chain
LGTM. Verify compatibility with other modules.
The addition of the indirect dependency cosmossdk.io/schema v0.3.1-0.20240930054013-7c6e0388a3f9 aligns with the PR objective of fixing an API break. This pre-release version likely contains necessary updates to resolve the issue.
To ensure this change doesn't introduce compatibility issues, please run the following script to check if other modules are using compatible versions of the schema package:
If any inconsistencies are found, consider updating those modules to use the same version of the schema package to maintain compatibility across the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of cosmossdk.io/schema across modules
# Test: Search for cosmossdk.io/schema usage in go.mod files
echo "Checking cosmossdk.io/schema versions across modules:"
find . -name go.mod -exec grep -H "cosmossdk.io/schema" {} +
# Test: Verify that no other module is using an older version
echo "Modules using older versions of cosmossdk.io/schema:"
find . -name go.mod -exec grep -H "cosmossdk.io/schema v0.3.0" {} +
Length of output: 4935
| ABCIListeners: []storetypes.ABCIListener{listenerWrapper{listener.Listener}}, | ||
| StopNodeOnErr: true, | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
listenerWrapper does not implement the required ABCIListener interface methods.
- The
listenerWrapperstruct does not define theListenFinalizeBlockandListenCommitmethods required by theABCIListenerinterface. - Consider implementing these methods within
listenerWrapperto ensure correct functionality.
🔗 Analysis chain
Ensure Correct Implementation of listenerWrapper
The listenerWrapper is initialized with listener.Listener. Verify that listener.Listener correctly implements the necessary interfaces and that wrapping it is required for the desired functionality.
Run the following script to confirm the implementation details:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if listener.Listener implements the required interface methods.
# Test: List the methods of listener.Listener. Expect: Methods matching the expected interface.
ast-grep --lang go --pattern $'type Listener interface {
$$$
}' --json
Length of output: 2316
Script:
#!/bin/bash
# Description: Retrieve the storetypes.ABCIListener interface methods.
# Extract the ABCIListener interface definition and list its methods.
ast-grep --lang go --pattern $'type ABCIListener interface {
$$$
}' --json
Length of output: 2512
Script:
#!/bin/bash
# Description: List all methods implemented by listenerWrapper.
# Extract the listenerWrapper struct and list its methods.
ast-grep --lang go --pattern $'type listenerWrapper struct {
$$$
}' --json
# List methods with receiver listenerWrapper
ast-grep --lang go --pattern $'func (lw listenerWrapper) $$$' --json
Length of output: 1101
Co-authored-by: Julien Robert <[email protected]>
* main: chore(baseapp): fix api break latest schema (#21978)
Description
Fixes small api break in baseapp due to #21636
Resolves: https://github.com/cosmos/nightly-stack/actions/runs/11067409014/job/30750713185
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores